Skip to content

Replace print() calls in agents.py with structured logging (Fixes #75)#85

Open
pragati-0208 wants to merge 2 commits intoINCF:mainfrom
pragati-0208:fix-agents-logging
Open

Replace print() calls in agents.py with structured logging (Fixes #75)#85
pragati-0208 wants to merge 2 commits intoINCF:mainfrom
pragati-0208:fix-agents-logging

Conversation

@pragati-0208
Copy link
Contributor

Summary

Replaced print() statements in agents.py with Python's logging module to provide structured and configurable logging.

Changes

  • Added logger initialization:
    logger = logging.getLogger("agents")

  • Replaced all print() calls with appropriate logging levels:

    • logger.info() for normal workflow messages
    • logger.error() for error conditions
    • logger.exception() for exception tracebacks

@pragati-0208
Copy link
Contributor Author

Hi! I have resolved the merge conflicts and updated the PR. Please let me know if any further changes are needed.

Copy link
Collaborator

@QuantumByte-01 QuantumByte-01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logging conversion itself is correct (good use of %s/%d lazy formatting instead of f-strings). But there are two bugs introduced:

1. Missing return in exception handler (critical)
Same issue as PR #92return f"Error: {e}" is removed with no replacement:

except Exception as e:
    logger.error("Error in handle_chat: %s", e)
    logger.exception("Exception occurred in handle_chat")
    # ← no return — handle_chat returns None on exception

Please add: return "I encountered an error. Please try again."

2. Removes try/except around call_gemini_for_final_synthesis (regression)
The original had a fallback for Gemini failures:

# Original (safe):
try:
    response = await call_gemini_for_final_synthesis(...)
except Exception:
    response = "Unable to process your request. Please try again."

# PR #85 (unsafe):
response = await call_gemini_for_final_synthesis(...)  # unguarded

If Gemini returns an error, the exception now bubbles up unhandled. Please restore the try/except.

3. Indentation on multi-line logger calls (minor)

# Wrong:
logger.info(
"Results summary: KS=%d, Vector=%d, Combined=%d",
len(ks_results),
)

# Correct:
logger.info(
    "Results summary: KS=%d, Vector=%d, Combined=%d",
    len(ks_results),
)

Fix these three and this PR is good to merge.

@pragati-0208
Copy link
Contributor Author

Fixed all review comments:

  • Restored missing return in exception handler
  • Added try/except around Gemini synthesis call
  • Fixed logger indentation

Also added handling for empty results.

Ready for review.

@QuantumByte-01
Copy link
Collaborator

Thanks for the update! However, no new commits have been pushed to this branch — the PR still points to the same commit from March 13, which is before our review. Could you push the fixes so we can re-review the actual code changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants